Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add $macc_v2 #4818

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from
Draft

Add $macc_v2 #4818

wants to merge 3 commits into from

Conversation

povik
Copy link
Member

@povik povik commented Dec 13, 2024

What are the reasons/motivation for this change?

$macc is needlessly complex: the CONFIG parameter uses a convoluted encoding, and it has a redundant B port for single-bit summands.

With $macc_v2 the goal is to make it simpler, so much so that it's not unreasonable to write techmap rules matching on this cell in plain Verilog (as opposed to just deferring to maccmap).

Explain how this is achieved.

Changes done in v2:

  • use "terms" instead of "ports" to label individual summands (to remove confusion with RTLIL-level ports)
  • use simple encoding for parameters (akin to $mem_v2, which similarly encodes a dynamic number of read/write ports)
  • remove the original B port semantics, instead use A/B ports to split the two operands for each product

Based off #4817, which can go in first to make this one easier to review.

TODO:

  • decide on the final shape of $macc_v2
  • add satgen support
  • add consteval support
  • fix techmap.v
  • test all basic scenarios
  • add simlib model
  • find the "adding a new cell" checklist
  • make booth work
  • Add to kernel/celltypes.h (incl. eval() handling for non-mem cells)
  • Add to InternalCellChecker::check() in kernel/rtlil.cc
  • Add to techlibs/common/simlib.v
  • Add to techlibs/common/techmap.v
  • Add to docs/source/CHAPTER_CellLib.rst (or just add a fixme to the bottom)
  • Maybe add support to the Verilog backend for dumping such cells as expression

The B port is for single-bit summands. These can just as well be
represented as an additional summand on the A port (which supports
summands of arbitrary width). An upcoming `$macc_v2` cell won't be
special-casing single-bit summands in any way.

In preparation, make the following changes:

 * remove the `bit_ports` field from the `Macc` helper (instead add any
   single-bit summands to `ports` next to other summands)

 * leave `B` empty on cells emitted from `Macc::to_cell`
@povik
Copy link
Member Author

povik commented Dec 13, 2024

to write techmap rules matching on this cell in plain Verilog (as opposed to just deferring to maccmap).

This use case is better served with having an extra port of plain summands, and leave A and B ports for products

So that you have

  Y = A_1*B_1 + A_2*B_2 + ... + A_n*B_n + C_1 + ... + C_m

where each A_j, B_j, C_j are an extracted bit range from A, B, C input ports respectively

Cc @phsauter also @widlarizer @whitequark (we touched on $macc semantics in #4316)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant